Skip to content

Feature/itk volume conversions#394

Draft
mccle wants to merge 13 commits intov0.28.0devfrom
feature/itk_volume_conversions
Draft

Feature/itk volume conversions#394
mccle wants to merge 13 commits intov0.28.0devfrom
feature/itk_volume_conversions

Conversation

@mccle
Copy link
Copy Markdown
Collaborator

@mccle mccle commented Feb 20, 2026

Depends on #387 for optional dependency importing.

Introduces 'to' and 'from' methods for the itk and SimpleITK libraries on the highdicom.Volume class.

channels=self._channels,
)

def to_sitk(self) -> SimpleITK.Image:
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this type hint will fail at runtime if SimpleITK is not installed. I think this would work though:

Suggested change
def to_sitk(self) -> SimpleITK.Image:
def to_sitk(self) -> 'SimpleITK.Image':

Comment on lines +3617 to +3620
def from_sitk(
cls,
sitk_im: SimpleITK.Image
):
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should expose frame_of_reference_uid and coordinate_system as optional parameters (with default parameters None and PATIENT respectively) and pass them through to the new volume

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also missing type hint on the return

SimpleITK.Image:
Image constructed from the Volume.

"""
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice to add some more information to the docstring to help users:

  • SITK (itk) uses the same LPS convention as highdicom
  • The returned volume is transposed to hide the difference in row major / column major orderings between the two formats transparent from the user

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants